-
Notifications
You must be signed in to change notification settings - Fork 280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(repo): Replace clerk.dev with clerk.com #1878
Conversation
🦋 Changeset detectedLatest commit: 9fe869b The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ee766af
to
66e547c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff! Just a small change!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (only one note regarding backwards compatibility)! 🚀
@@ -1,4 +1,4 @@ | |||
export const API_URL = 'https://api.clerk.dev'; | |||
export const API_URL = 'https://api.clerk.com'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ I'm wondering if there could be use cases that depend on that value, for example:
- test suites asserting things like
expect(request).to('api.clerk.dev')
- strict firewalls configured to only allow outgoing traffic to api.clerk.dev and not api.clerk.com
These seem unlikely to me, but just wanted to point out that this has some small chance of breaking someone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strict firewalls configured to only allow outgoing traffic to api.clerk.dev and not api.clerk.com
@agis Could you check the firewall rules if we support the clerk.com for all of our workers services (eg img.clerk.com)? If so, I think we could proceed with this PR.
test suites asserting things like expect(request).to('api.clerk.dev')
Could you elaborate more on this? In the javascript repo, I believe that all the tests are updated to use clerk.com
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was referring to customers' test suites and WAF, not our own (our own WAF is fine, since clerk.com
is already used by a handful of customers already). I was just trying to envision any cases where changing this constant's value would break customers.
Feel free to disregard my comment if you believe this is highly unlikely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the above comments i think we should introduce the change of clerk.dev
-> clerk.com
) as part of the next major version and add references in the migration guide and changelog about changing the clerk domains to avoid causing issues in customers depending the requests to our systems (eg mocking the requests).
66e547c
to
d5c9fb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Blocked by #1837 |
d5c9fb7
to
0f6c826
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup @dimkl
0f6c826
to
84537e0
Compare
84537e0
to
9fe869b
Compare
This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Since we have completely migrated to clerk.com, this PR replaces all references of the previous domain (
clerk.dev
) toclerk.com
.Checklist
npm test
runs as expected.npm run build
runs as expected.Type of change
Packages affected
@clerk/clerk-js
@clerk/clerk-react
@clerk/nextjs
@clerk/remix
@clerk/types
@clerk/themes
@clerk/localizations
@clerk/clerk-expo
@clerk/backend
@clerk/clerk-sdk-node
@clerk/shared
@clerk/fastify
@clerk/chrome-extension
gatsby-plugin-clerk
build/tooling/chore